-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Add Syndication Links support #331
base: develop
Are you sure you want to change the base?
Conversation
@Firestorm980 thanks for the PR! Could you please fill out the PR template with description, changelog, and credits information so that we can properly review and merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Firestorm980,
Thanks a lot for working on this and raising the PR. Great work here. PR looks good but I have noticed one issue during testing: when we publish the post and it gets autoposted on X/Twitter, I didn't see syndication links added to the post (even after refreshing). Additionally, when I retweet, I find the syndication links filled up with the first input blank. Please check the attached video for more information on this.
Screen.Recording.2024-07-05.at.8.26.02.PM.mov
Could you please help check if this is happening on your end as well?
Thanks,
👋🏼 @dshanske would love your review here as well |
Will do |
It looks good to me. Makes me think I should just give in and write a block compatible version...but...maybe this will inspire someone to submit a PR or two. To be honest. the concept of the plugin is a nice and simple...store and display links philosophy. |
@jeffpaul I can replicate the same issue with the PR as mentioned by @iamdharmesh |
…ere we have status and detail properties instead of an errors array. Fixes PHP Warning: Undefined property: stdClass::$errors happening when $response->errors not exists.
@jeffpaul I could also replicate what @iamdharmesh mentions. Currently, when using the Classic Editor, the action to set the syndication links is only triggered when the "Post to X/Twitter" button is clicked, and not when a new post is published, as it is only called in the In my opinion, there are the following adjustments that could be made:
I've started working on the above on my local setup but it will take some time. I will try to find some next week, and push a POC with my suggestions. |
Happy to accept anything upstream if it would help |
… with late priority. Syndication Links plugin initialization is at 11. Otherwise, class_exists( 'Syn_Meta' ) will evaluate to false since it doesn't still exists in context.
…us_updated action.
…andle syndication links.
…upport integration currently in progress on: 10up/autoshare-for-twitter#331.
Over the past few days, I spent quite a bit of time debugging and ended up hitting the same conclusions that @gsarig mentioned above. In my own environment, I initially noticed that calls to After digging deeper, I also discovered that Syndication Links itself attempts some integration along similar lines (though it has some additional issues, which I documented in the PR I just submitted upstream. Once I resolved that conflict, I created a separate class to have a single source of truth for handling syndication links, and also extended the However, I’m still seeing a puzzling behavior when the post is first published:
I temporarily added debug output in Here’s the relevant portion of // Log the final state before update
error_log( 'FINAL UPDATE METADATA CALL - Type: ' . $type );
error_log( 'FINAL UPDATE METADATA CALL - ID: ' . $id );
error_log( 'FINAL UPDATE METADATA CALL - Meta key: mf2_syndication' );
error_log( 'FINAL UPDATE METADATA CALL - Links: ' . print_r( $links, true ) );
// Let's check if the meta key already exists
$existing_meta = get_metadata( $type, $id, 'mf2_syndication', true );
error_log( 'EXISTING META BEFORE UPDATE: ' . print_r( $existing_meta, true ) );
$result = update_metadata( $type, $id, 'mf2_syndication', $links );
error_log( 'UPDATE METADATA RESULT: ' . ( $result ? 'true' : 'false' ) );
// Double check if it was actually saved
$check_meta = get_metadata( $type, $id, 'mf2_syndication', true );
error_log( 'META AFTER UPDATE CHECK: ' . print_r( $check_meta, true ) );
return $result; Here is the first metadata update call (happening immediately on post creation and the first tweet). Notice how it says "UPDATE METADATA RESULT: true" and "META AFTER UPDATE CHECK" logs out the correct link:
Then, here’s the second update call (when I do a retweet via the "Post to X/Twitter now" UI, sending out the second tweet). This time, the logs show that it retrieved the array containing the first link, appends the second link, and again says
I'm attaching screenshots below to give greater visual context and support my reproduction: 1st tweet - Missing 2nd tweet - Saves the I’m not sure if this is related to a save-post race condition, or if some other part of the Syndication Links plugin lifecycle is rolling back metadata changes during the first publish, but I’m looking into that further. If anyone else can replicate this behavior or has ideas on what might be interfering with the creation of that metakey on first publish, please let me know! In the meantime, feel free to test what’s currently in the PR—I’d love to see if others get the same results or can cast some light on this odd behavior. |
Description of the Change
Adds support for Syndication Links with autoshare. Automatically adds the links to Twitter/X to Syndication Links meta.
Closes #79
How to test the Change
Notes:
Changelog Entry
Credits
Props @Firestorm980
Checklist: